Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MessageBus restructure #162

Draft
wants to merge 49 commits into
base: master
Choose a base branch
from
Draft

MessageBus restructure #162

wants to merge 49 commits into from

Conversation

ChiTimesChi
Copy link
Collaborator

Description

Replaces #158 #161

This PR aims to provide an immutable MessageBus, which is supposed to serve as a backbone for the messaging infra on all the chains. All potential upgradeable logic is separated into separate contracts:

  • AuthVerifier is responsible for verifying the submitted messages, which can be later upgraded to verifying messages using a submitted proof.
  • MessageExecutor is responsible for executing messages of any types, as well as for calculating the fees for doing so from a remote chain.

MessageBus is responsible for:

  • Sending messages:
    • Applying correct fee from MessageExecutor.
    • Emitting events, indicating that message was sent.
  • Receiving messages:
    • Checking that every message can be executed only once.
    • Checking that message sender is authenticated by AuthVerifier.
    • Passing message correctly to have it executed by MessageExecutor.

Checklist

  • New Contracts have been tested
  • Lint has been run
  • I have checked my code and corrected any misspellings

@ChiTimesChi
fix GasFeePricing interface
6c6e9f5
@ChiTimesChi
GasFeePricing: first draft
0b21dbb
@ChiTimesChi
chore: formatting + docs
6562105
@ChiTimesChi
estimateGasFees for a bunch of messages
36c0372
@ChiTimesChi
_send(): multiple messages at a time
ba0cb18
@ChiTimesChi
Cross-chain gas price setup: first draft
03e53e2
@ChiTimesChi
GasFeePricing: messages to update chain info on all other chains
69dc324
@ChiTimesChi
GasFeePricing: manual setDstChainConfig
dbfbd43
@ChiTimesChi
estaimateFee should be view
775783a
@ChiTimesChi
add src-messaging to remappings
42c5c47
@ChiTimesChi
update test Utilities
548766b
@ChiTimesChi
tests: access, initialized for GasFeePricing
b1249fa
@ChiTimesChi
tests: prepare for setters/getters testing
42c5cf0
@ChiTimesChi
I like pretty boxes
b6c6b31
@ChiTimesChi
GasFeePricing: min fee for gas usage on dst chain
25829f5
@ChiTimesChi
GasFeePricing: better docs
73d8273
@ChiTimesChi
markups now start from 0%, not 100%
a6f1783
@ChiTimesChi
I strive for perfection
4e3ff05
@ChiTimesChi
GFP: markups are now chain-specific
869d3ab
@ChiTimesChi
GFP: fix encoding
e17a6ed
@ChiTimesChi
GFP: remove duplicated functions, refactor names
44ec353
@ChiTimesChi
tests: check minGasUsageFee post-initialization
8e44415
@ChiTimesChi
setTrustedRemotes for a bunch of chains
5580db9
@ChiTimesChi
GFP: remove dstGasFeePricing(was shadowing trustedRemoteLookup)
74fe440
@ChiTimesChi
GFP: allow to set zero gas unit price (hi Aurora)
a58544c
@ChiTimesChi
GFP: add missing checks, unify ordering
5d14361
@ChiTimesChi
tests: GFP setters
cd88583
@ChiTimesChi
GFP: refactoring + better docs
221e204 
- variable names should be more clear now
- enforced alfabetical order of variables to avoid confusion
- docs on the srtucts are more verbose
- tests are updated to reflect the these changes
@ChiTimesChi
refactor: src/dst -> local/remote
ec5a5ec
@ChiTimesChi
GFP: introduce remote chain-specific min fee
4059476
@ChiTimesChi
tests: split GFP tests into categories
ae006ee
@ChiTimesChi
GFP: implement (remote chain)-specific min fee
94675e9
@ChiTimesChi
gas fee pricing explained
aa44c15
@ChiTimesChi
GFP: remote ratios are no longer used
0c8e461
@ChiTimesChi
tests: sending/receiving messages by GasFeePricing
635d984
@ChiTimesChi
GFP: fix bugs in estimateGasFee:
7530588 
- wrong value was used to check for maxGasDrop
- some values were not cast to uin256, resulting in overflows
@ChiTimesChi
tests: GFP minFee, applying markups
66dc56e
@ChiTimesChi
MB: immutable contracts
6f1d697
@ChiTimesChi
slightly better docs
c47eca8
@ChiTimesChi
remove extra casting
3757cd5
@ChiTimesChi
remove unnecessary cast to uint64
4830921
@ChiTimesChi
chore: rename new libraries
88e4a80
@ChiTimesChi
yoink bytes32 to address library
4a836b0
@ChiTimesChi
revert on encoding msg options, not on decoding
ca2a403
@ChiTimesChi
GasFeePricing -> MessageExecutor
969607f
@ChiTimesChi
Merge branch 'feature/message-bus-immutable' into feature/message-bus…
c75f2e1 
…-restructure
@ChiTimesChi
Merge branch 'feature/messaging-gas-pricing' into feature/message-bus…
5bb3cf2 
…-restructure
This was referenced May 27, 2022
Closed
Closed
@coveralls
Copy link

coveralls commented May 27, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2398388162

  • 0 of 190 (0.0%) changed or added relevant lines in 10 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.1%) to 24.744%
Changes Missing Coverage Covered Lines Changed/Added Lines %
contracts/messaging/ContextChainId.sol 0 2 0.0%
contracts/messaging/HarmonyMessageBus.sol 0 2 0.0%
contracts/messaging/libraries/Bytes32AddressLib.sol 0 2 0.0%
contracts/messaging/MessageBus.sol 0 9 0.0%
contracts/messaging/libraries/OptionsLib.sol 0 9 0.0%
contracts/messaging/libraries/PricingUpdateLib.sol 0 11 0.0%
contracts/messaging/framework/SynMessagingReceiverUpgradeable.sol 0 14 0.0%
contracts/messaging/MessageBusReceiver.sol 0 16 0.0%
contracts/messaging/MessageBusSender.sol 0 19 0.0%
contracts/messaging/MessageExecutorUpgradeable.sol 0 106 0.0%
Totals Coverage Status
Change from base Build 2386183733: -2.1%
Covered Lines: 664
Relevant Lines: 2474
💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ChiTimesChi @coveralls @aureliusbtc